Skip to content

Fix hashtable .Keys/.Values collision in skill coverage tool - #831

Merged
Evangelink merged 1 commit into
mainfrom
evangelink-fix-skill-coverage-keys-collision
Jun 25, 2026
Merged

Fix hashtable .Keys/.Values collision in skill coverage tool#831
Evangelink merged 1 commit into
mainfrom
evangelink-fix-skill-coverage-keys-collision

Conversation

@Evangelink

Copy link
Copy Markdown
Member

What

Get-SignificantTerms in eng/skill-coverage/Measure-SkillCoverage.ps1 returned $terms.Keys. When a teaching point's text contains the word "keys", PowerShell's hashtable member access resolves .Keys to that entry's value ($true) instead of the key collection — so the keyword set collapses to {True} and the point can never meet the ≥2-hit match rule.

This made dotnet-test/test-tagging Step 3 "Classify each test method" permanently uncoverable, regardless of eval content.

Fix

Use the method form (get_Keys() / get_Values()), which bypasses key-name shadowing. Applied to the confirmed .Keys return and the two same-class .Values returns for robustness.

Verification

$h=@{}; $h['keys']=$true; @($h.Keys)        # -> True   (bug)
$h=@{}; $h['keys']=$true; @($h.get_Keys())  # -> keys   (fixed)

After the fix, test-tagging coverage goes 24→25/28 (Step 3 now credited) with no regressions across the rest of the dotnet-test plugin. Complements #830, whose author surfaced this bug.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Get-SignificantTerms returned $terms.Keys; when a teaching point contains the word 'keys', PowerShell hashtable member access resolves to that entry's value instead of the key collection, collapsing the keyword set and making points like test-tagging Step 3 uncoverable. Use get_Keys()/get_Values() to bypass key-name shadowing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 25, 2026 08:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a PowerShell hashtable member-access edge case in the skill coverage measurement script where a literal significant term like "keys" could shadow the .Keys property and collapse the extracted keyword set, preventing certain teaching points from ever being credited.

Changes:

  • Replace hashtable property access .Keys / .Values with get_Keys() / get_Values() to avoid key-name shadowing.
  • Add an inline comment explaining the .Keys collision scenario and why the method form is used.
Show a summary per file
File Description
eng/skill-coverage/Measure-SkillCoverage.ps1 Uses get_Keys() / get_Values() to prevent keyword extraction and pattern enumeration from breaking when a hashtable contains entries named keys/values.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@Evangelink
Evangelink enabled auto-merge (squash) June 25, 2026 09:00
@github-actions github-actions Bot added the waiting-on-review PR state label label Jun 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Evaluation passed for ee46ffc. cc @ViktorHofer @JanKrivanek — please review.

@Evangelink
Evangelink merged commit 7d4f9fd into main Jun 25, 2026
36 checks passed
@Evangelink
Evangelink deleted the evangelink-fix-skill-coverage-keys-collision branch June 25, 2026 10:42
elvisw pushed a commit to elvisw/dotnet-skills that referenced this pull request Jun 29, 2026
…dotnet#831)

Get-SignificantTerms returned $terms.Keys; when a teaching point contains the word 'keys', PowerShell hashtable member access resolves to that entry's value instead of the key collection, collapsing the keyword set and making points like test-tagging Step 3 uncoverable. Use get_Keys()/get_Values() to bypass key-name shadowing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-review PR state label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants